Skip to content

refactor: move the schema source modules into @modelcontextprotocol/core#2477

Open
felixweinberger wants to merge 7 commits into
mainfrom
fweinberger/core-schemas-home
Open

refactor: move the schema source modules into @modelcontextprotocol/core#2477
felixweinberger wants to merge 7 commits into
mainfrom
fweinberger/core-schemas-home

Conversation

@felixweinberger

Copy link
Copy Markdown
Contributor

Moves the neutral schema modules — the spec schemas, the OAuth/OpenID schemas, and the protocol constants — out of core-internal and into @modelcontextprotocol/core as real source. client, server, and server-legacy now resolve them from core as a regular (exact-pinned) runtime dependency instead of each bundling a private copy.

Motivation and Context

#2459 deduplicated the schema graph by rewriting module ids to the core specifier at build time. That produced the right artifacts, but the dependency direction lived only inside three build configs — source still said the schemas belonged to core-internal, and a rename or split could silently re-inline ~2,400 lines of Zod graph into every consumer with all gates green. Moving the source makes the published edge real: core owns the schemas, core-internal forwards from the old paths (one-line-per-name shims; a test keeps them forwarding-only), and the bundlers externalize a declared dependency with no plugin at all. Supersedes #2459.

An application importing more than one of the packages evaluates one schema graph instead of two or three, with shared object identity across packages.

How Has This Been Tested?

  • Full workspace suites (3,881 tests) including integration and the Cloudflare Workers e2e, run over packed tarballs.
  • Dist comparison against the perf: resolve schema modules from @modelcontextprotocol/core instead of inlining them #2459 approach: bare specifier in both ESM and CJS, schema bodies absent from consumer bundles, public export surfaces byte-identical (the 172-export pin is untouched); the constants module is additionally externalized.
  • Scratch npm consumer over packed tarballs: live client↔server session; npm resolves core transitively and dedupes to a single core and single zod; schemas imported from core are the same objects the packages use internally.
  • A real bundling consumer measured a cold-start improvement identical to (marginally better than) the perf: resolve schema modules from @modelcontextprotocol/core instead of inlining them #2459 mechanism.
  • New guard tests: a client dist boundary test asserting both directions (every name imported from core resolves against core's export map; schema bodies never defined locally), and a shim test asserting the forwarding files contain no zod.

Breaking Changes

None. Public APIs, types, and behavior are unchanged. Additive: core gains a ./internal subpath (documented as an SDK-internal contract that may change in any release); the three packages gain core as an exact-pinned dependency, resolved automatically by npm. core/client/server/server-legacy now version together via a changesets fixed group, so a mismatched pair can never be published or installed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

  • The schema modules moved verbatim (git detects all three as 100%-similarity copies); the real change is ~380 lines of shims, wiring, and tests.
  • The frozen wire-era modules are untouched — the constants shim exists precisely so they keep a stable import path.
  • Publish coupling is deliberate and enforced: the fixed group + exact pins mean core always releases with its consumers; a version skew would fail at import time with ERR_PACKAGE_PATH_NOT_EXPORTED rather than drifting silently.
  • dts note: schema types are still emitted per package as before — this PR deduplicates runtime evaluation only.
  • Follow-up candidates: extend the dist boundary test to server/server-legacy; narrow ./internal once the barrel export-star cleanup lands.

The MCP spec schemas (types/schemas.ts), the OAuth/OpenID schemas
(shared/auth.ts), the protocol constants (types/constants.ts), and the three
JSON value types move verbatim from core-internal into packages/core/src —
core now owns the schema sources instead of bundling them out of
core-internal through build-only subpath aliases.

Mechanism:
- core's curated root entry re-exports the same public surface as before
  (unchanged 172 exports), now from its own local modules.
- A new ./internal subpath re-exports the moved modules wholesale for the
  sibling SDK packages (internal helper schemas, auth types, constants, JSON
  types — names that are deliberately not public on the root entry).
- core-internal keeps the old module paths as one-to-one named re-export
  shims, so no importer or test changes anywhere.
- client/server/server-legacy declare @modelcontextprotocol/core as a real
  dependency; their bundles keep @modelcontextprotocol/core/internal as an
  external runtime import (explicit tsdown external entry) instead of
  carrying their own bundled schema copies.
- The build-only core-internal/schemas + core-internal/auth tsconfig/tsdown
  aliases are deleted everywhere; the per-package tsconfigs gain a single
  source-first alias for @modelcontextprotocol/core/internal so typecheck
  and vitest stay build-order independent.

Layering stays acyclic: core depends only on zod; core-internal depends on
core; the wire era modules are untouched (their frozen copies and their
runtime constants imports resolve through the shims unchanged).
…home

Test-only adaptations to the schema-source move; each guard keeps pinning the
same invariant, only pointed at the new canonical location:

- packageTopologyPins: pin @modelcontextprotocol/core's export map as
  ['.', './internal'] — the internal seam is deliberate, not public API.
- coreSchemas: read the spec-schema source from core's own src/schemas.ts
  (the auth group still reads core-internal's authSchemas registry).
- wireOnlyHiding: read the @deprecated task-schema and constants sources
  from packages/core/src (the old paths are now re-export shims with no
  doc comments to scan).
- codemod authSchemaNames: core's barrel now ends its auth block with
  "} from './auth'" instead of the deleted build-only alias specifier.
- cloudflareWorkers: generalize packServerPackage to packWorkspacePackage
  and install the workspace core tarball alongside the server tarball —
  the packed server resolves @modelcontextprotocol/core/internal at
  runtime, which the registry copy of core does not carry yet.
The schema sources moved into @modelcontextprotocol/core with the sibling
packages resolving @modelcontextprotocol/core/internal at runtime. That seam
can rot in ways no existing test catches; this locks it down:

- Exact sibling pins: client/server/server-legacy (and core-internal, for
  consistency) depend on core via workspace:* so pnpm publishes an exact
  version pin instead of a caret range. The ./internal surface is only
  guaranteed for the core version each sibling was built against, so a caret
  range would let installs mix skewed versions.
- Changesets fixed group for core + client + server + server-legacy, keeping
  the pinned versions releasable in lockstep.
- ./internal is labeled as an SDK-internal contract in its header, with
  @internal JSDoc on the re-exports (source-level only: the dts bundler
  flattens the re-exports and drops statement-level comments, so the built
  declarations do not carry the tag).
- A client boundary test parses the built dists in both directions: every
  name the client bundle imports or re-exports from core must resolve
  against core's built entry export lists (skew), and sentinel schemas that
  exist only in core's modules must never appear as definitions in the
  client bundle (re-inlining via lost external config or eager aliases).
- A core-internal shim-purity test pins the old schema/auth/constants module
  paths as pure re-export forwards: no zod import, no local definitions,
  imports only from @modelcontextprotocol/core/internal — so new schemas
  can't accrete at the old paths and silently miss core's published entries.
  The shim headers now state that rule, and the type-only JSON value
  re-export in types.ts is pinned as erasable.

No-Verification-Needed: tests, manifest pins, release config, and comments only — no runtime surface change
No-Verification-Needed: release-metadata-only change
@felixweinberger felixweinberger requested a review from a team as a code owner July 10, 2026 17:18
@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 68688c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@modelcontextprotocol/core Minor
@modelcontextprotocol/client Minor
@modelcontextprotocol/server Minor
@modelcontextprotocol/server-legacy Minor
@modelcontextprotocol/core-internal Patch
@modelcontextprotocol/express Major
@modelcontextprotocol/fastify Major
@modelcontextprotocol/hono Major
@modelcontextprotocol/node Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Jul 10, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2477

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2477

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2477

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2477

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2477

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2477

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2477

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2477

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2477

commit: 68688c9

@pkg-pr-new

pkg-pr-new Bot commented Jul 10, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2477

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2477

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2477

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2477

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2477

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2477

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2477

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2477

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2477

commit: e55dc27

barrelClean and coreBoundary each rebuilt a missing dist from their own
beforeAll; vitest runs test files in separate workers, so a cold checkout
raced two pnpm builds in the same package and the clean step deleted
files under the other worker's reader.

Route both through a shared single-flight helper: an atomic mkdir lock
with one canonical sentinel set per package, a stale-lock steal for
holders that died without cleanup, an async build bounded by a timeout
so the worker's event loop and vitest's hook timer stay live, and only
EEXIST treated as contention.

No-Verification-Needed: test-only change, no runtime surface
@felixweinberger

Copy link
Copy Markdown
Contributor Author

@claude review

The builds take seconds; the previous bounds were sized by stacking
worst cases on worst cases. Keep the ordering invariant (build kill <
waiter deadline < stale-lock steal < hook timeout) at realistic
magnitudes: 60s/90s/120s with 180s and 240s hooks.

No-Verification-Needed: test-only timeout constants
Comment thread packages/core/package.json
Comment thread packages/client/package.json
Comment thread packages/client/test/client/coreBoundary.test.ts Outdated
@felixweinberger

Copy link
Copy Markdown
Contributor Author

Reopening as a fresh PR to retrigger the automated review.

… blind spots

- Add typesVersions for the ./internal subpath so moduleResolution:node10
  consumers resolve its declarations (exports maps are invisible there)
- Update wire-schemas and packages docs: core now arrives transitively as
  the shared runtime schema graph of client/server/server-legacy
- Boundary test: scan dist recursively (validators/ chunks were excluded)
  and reject bare side-effect imports of core, which have no from-clause
@felixweinberger

Copy link
Copy Markdown
Contributor Author

@claude review

Comment on lines +1 to +6
// Moved: the spec Zod schemas now live in @modelcontextprotocol/core (packages/core/src/schemas.ts).
// This module re-exports them one-to-one so every existing import path keeps working; a name
// added to core/src/schemas.ts must be added here too (specTypeSchema.ts imports through this
// module, so a missing name fails typecheck loudly).
//
// Add new schemas in packages/core/src/schemas.ts, never here — this file only forwards, and

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The frozen-wire-schema guards are still keyed only to the old module path: the eslint no-restricted-imports group in packages/core-internal/eslint.config.mjs (['**/types/schemas', '**/types/schemas.js']) and the RUNTIME_SCHEMAS_IMPORT regex in packages/core-internal/test/wire/layeringInvariants.test.ts:50 both require the literal substring types/schemas, so a src/wire/rev*/ file importing the same mutable schemas via the PR's new @modelcontextprotocol/core/internal seam would lint clean, pass the invariant test, typecheck, and resolve at runtime. No wire/rev file does this today, so this is a guard coverage gap rather than a live bug — extending both patterns to also match @modelcontextprotocol/core and @modelcontextprotocol/core/internal closes it.

Extended reasoning...

What the gap is

The repo enforces a frozen-wire-schema invariant — "A change to types/schemas.ts must never alter what a codec emits or accepts on the wire" (the comment above the rule in packages/core-internal/eslint.config.mjs:34-37) — in two places, and both are keyed to the schemas' old home only:

  1. eslint (packages/core-internal/eslint.config.mjs:38-55): for src/wire/rev*/**/*.ts, @typescript-eslint/no-restricted-imports restricts only the group ['**/types/schemas', '**/types/schemas.js'].
  2. layeringInvariants (packages/core-internal/test/wire/layeringInvariants.test.ts:50): RUNTIME_SCHEMAS_IMPORT = /^(import|export)\s+(?!type\b)[^;]*?\bfrom\s+['\"][^'\"]*types\/schemas(?:\.js)?['\"]/m, applied to every file under src/wire/rev*/ by invariant (b) — a test that exists explicitly so "the lint rules cannot be silently weakened".

Neither pattern matches @modelcontextprotocol/core/internal or @modelcontextprotocol/core — both require the literal substring types/schemas in the module specifier.

Why this PR opens the bypass

Before the move, the dependency direction was core→core-internal, so no @modelcontextprotocol/core specifier could even resolve from inside core-internal. This PR reverses that: @modelcontextprotocol/core becomes a runtime dependency of core-internal (packages/core-internal/package.json), and packages/core-internal/tsconfig.json gains a package-wide paths alias for @modelcontextprotocol/core/internalcore/src/internal.ts. core/src/internal.ts does export * from './schemas', so the new specifier is a second canonical path to the exact same mutable schema objects the wire-isolation guards exist to freeze against.

Step-by-step proof

  1. A contributor adding 2026-era wire support writes, inside src/wire/rev2026-07-28/:
    import { ContentBlockSchema } from '@modelcontextprotocol/core/internal';
    instead of freezing a copy into the rev directory.
  2. typecheck: passes — the tsconfig paths alias resolves the specifier to core/src/internal.ts.
  3. lint: passes — '@modelcontextprotocol/core/internal' matches neither '**/types/schemas' nor '**/types/schemas.js'.
  4. layeringInvariants (b): passes — the specifier contains no types/schemas substring, so RUNTIME_SCHEMAS_IMPORT doesn't match.
  5. runtime: works — core is a real dependency, and the bundles keep core/internal external.
  6. Later, a spec-tracking widening of ContentBlockSchema in core/src/schemas.ts retroactively changes what the supposedly-frozen 2026 era accepts on the wire — with every gate green. That is precisely the drift the invariant was written to prevent.

Why nothing else catches it

The PR's own new guards cover other directions but not this one: schemaShims.test.ts only pins the three re-export shims to be forwarding-only, and coreBoundary.test.ts only checks that names the client dist imports exist in core's built exports. A wire/rev file importing a perfectly valid mutable schema trips neither. Notably, the PR did update its other guard keyed to the schemas' location — wireOnlyHiding.test.ts now reads core/src/schemas.ts directly — so leaving the two wire-isolation guards on the retired path looks like an oversight rather than a design choice.

Impact and fix

I verified (grep) that no file under src/wire/ imports any @modelcontextprotocol/core specifier today, so nothing breaks on merge — this is future-drift insurance with a hole in it, not a live defect; hence nit rather than blocking. The fix is two one-line pattern extensions: add '@modelcontextprotocol/core' and '@modelcontextprotocol/core/internal' (plus subpath-glob variants as appropriate) to the eslint group in eslint.config.mjs, and extend RUNTIME_SCHEMAS_IMPORT in layeringInvariants.test.ts to also match the core specifiers, keeping the existing allowTypeImports/type-only exemptions.

Comment on lines +9 to +20
/**
* Canonical dist sentinels per package. Every caller waits on the same
* complete set: if callers checked their own subsets, a partial dist (from an
* interrupted build) could satisfy one caller's fast path while another still
* rebuilds — and the rebuild's clean step would wipe files under the first
* caller's running tests, which is exactly the race this helper exists to
* prevent.
*/
const DIST_SENTINELS: Record<string, string[]> = {
client: ['index.mjs', 'stdio.mjs'],
core: ['index.mjs', 'internal.mjs']
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Two robustness gaps in the new ensureBuilt helper: (1) DIST_SENTINELS.client = ['index.mjs', 'stdio.mjs'] is a strict subset of what the callers actually read (barrelClean.test.ts needs dist/index.d.mts/.d.cts, the shims*.mjs entries, and dist/validators/ajv.cjs), so a build interrupted after tsdown's JS bundling but before the slower tsc dts pass leaves a dist that satisfies haveAll() forever — the helper never rebuilds, and every later run fails with a confusing ENOENT until someone manually rebuilds. (2) The stale-lock steal is a non-atomic stat→rmSync sequence: two waiters can both observe the stale mtime, and the second's rmSync(force: true) deletes the first's fresh lock, letting two pnpm build runs (each with clean: true) execute concurrently — exactly the race this helper exists to prevent. Fix (1) by adding one sentinel per build phase (e.g. index.d.mts, shimsBrowser.mjs) or a completion-marker file written last, and (2) by making the steal atomic (e.g. renameSync the stale lock to a unique name; only the winner succeeds) or re-verifying staleness after acquiring.

Extended reasoning...

Issue 1: incomplete sentinel set can permanently wedge the dist

The helper's own docstring states the invariant: "Every caller waits on the same complete set: if callers checked their own subsets, a partial dist (from an interrupted build) could satisfy one caller's fast path…" — but the registered set itself is a subset of what the callers read. DIST_SENTINELS.client lists only index.mjs and stdio.mjs, while barrelClean.test.ts reads dist/index.d.mts and dist/index.d.cts (via rootExportBlockOf), dist/shimsNode.mjs / shimsWorkerd.mjs / shimsBrowser.mjs, and requires dist/validators/ajv.cjs; coreBoundary.test.ts reads every top-level *.mjs chunk.

Step-by-step failure: (1) On a cold checkout, worker A takes the lock and runs pnpm build. tsdown emits the rolldown JS bundles (including both sentinels) quickly; the resolver: 'tsc' dts pass is a distinctly slower, later phase. (2) The build is killed mid-flight — by the helper's own BUILD_TIMEOUT_MS = 60_000 execFile timeout, or a Ctrl-C propagating SIGINT to the child. The finally block still removes the lock. (3) dist now contains index.mjs + stdio.mjs but no declaration files. (4) On every subsequent run, haveAll() is true and no lock exists, so the fast path returns immediately — and even a worker that somehow takes the lock skips the build because !haveAll() is false. Per the design comment, "once the dist exists nobody ever rebuilds." (5) Every later test run fails with an ENOENT on index.d.mts (or a regex mismatch) — a confusing failure that points nowhere near the actual cause — and the state never self-heals until someone manually deletes dist/ or runs pnpm build.

Fix: include one artifact from each build phase in the sentinel list (e.g. add index.d.mts for the dts phase and shimsBrowser.mjs for the entry set), or have the build path write a completion-marker file as its last step and use that as the sole sentinel.

Issue 2: the stale-lock steal is not atomic

The steal path is stat → decide → rmSync(recursive, force)continuemkdirSync. The decision is made from a stat of the old stale lock, but rmSync unconditionally deletes whatever directory exists at lockDir at rm-time, with no re-check.

Step-by-step interleaving: a leftover .dist-build-lock from a previously killed run (SIGKILL/OOM — the documented scenario the steal exists for) means every vitest worker of the next run hits the steal path simultaneously at startup, since barrelClean.test.ts and coreBoundary.test.ts both call ensureBuilt(clientPkgDir) from separate parallel workers and all observe age > STALE_LOCK_MS at once. Then: (1) C stats the stale lock and decides to steal; (2) B stats it too, rmSyncs the old lock, continues, mkdirSyncs a fresh lock, and spawns pnpm build; (3) C — delayed between its stat and its rm — now executes rmSync(force: true), silently deleting B's fresh lock; (4) C's mkdirSync succeeds and C also spawns pnpm build. Two concurrent builds now run in the same package, and tsdown's clean: true wipes dist out from under the sibling build; worse, with B's lock gone, any third worker's fast path (haveAll() && !existsSync(lockDir)) can return and start reading a dist that C's clean step is about to delete. This is precisely the failure mode the file header says the helper prevents, reintroduced in its own recovery path.

Fix: make the steal atomic — e.g. renameSync the stale lock dir to a unique name (rename is atomic; only one stealer succeeds, losers get ENOENT and re-loop as ordinary waiters), or re-stat/verify the lock's mtime immediately after acquiring.

Why nit, not blocking

Both issues live in test-only infrastructure and fail loudly (ENOENT / a flaky red run), never as a silent false green, and nothing shipped to consumers is affected. Issue 1 requires an interrupted build on a local machine (CI runs from clean checkouts or pre-built dists) and recovers with one pnpm build; issue 2 additionally requires a dead-worker stale lock plus a microseconds-wide interleaving, and usually self-heals on the next run. But since this file's entire reason to exist is making on-demand builds race-free, both fixes are cheap and worth taking: they're each roughly a one-line change to freshly-written locking code.

## Reach for `core` only to validate raw wire JSON

`@modelcontextprotocol/core` exports the Zod schema constants the SDK validates protocol payloads against, for code that handles raw JSON-RPC payloads itself — gateways, proxies, log pipelines. Neither `server` nor `client` exports a Zod schema, and the matching TypeScript types ship with both, so if you only call `registerTool` and `callTool` you never install it. [Wire schemas](../advanced/wire-schemas.md) is the how-to.
`@modelcontextprotocol/core` exports the Zod schema constants the SDK validates protocol payloads against, for code that handles raw JSON-RPC payloads itself — gateways, proxies, log pipelines. Neither `server` nor `client` exports a Zod schema, and the matching TypeScript types ship with both, so if you only call `registerTool` and `callTool` you never import it directly — it arrives transitively, since `server` and `client` resolve their shared schema graph from it at runtime. [Wire schemas](../advanced/wire-schemas.md) is the how-to.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 A third instance of the stale core-topology claim survives in docs/migration/upgrade-to-v2.md:1328: it still says @modelcontextprotocol/core "is not required by client / server — install it only if you import the raw schemas directly", which is false after this PR makes core an exact-pinned runtime dependency. Commit 68688c9 fixed the same sentence in packages.md and wire-schemas.md but missed this one — reword it to match the corrected "arrives transitively as the shared runtime schema graph" language.

Extended reasoning...

What's stale. This PR makes @modelcontextprotocol/core an exact-pinned runtime dependency of client, server, and server-legacy (each package.json now declares "@modelcontextprotocol/core": "workspace:*"), and the built bundles resolve @modelcontextprotocol/core/internal as a live external import — pinned by the new coreBoundary.test.ts. The earlier review round flagged two docs asserting the old topology, and commit 68688c9 fixed both (docs/get-started/packages.md:69 and docs/advanced/wire-schemas.md:60 now say core "arrives transitively" as the shared runtime schema graph). But a third instance survived: docs/migration/upgrade-to-v2.md:1326-1329 still reads "It is runtime-neutral (its only dependency is zod) and is not required by client / server — install it only if you import the raw schemas directly."\n\nStep-by-step proof that the sentence is false post-merge. (1) A v1 user upgrading per this guide runs npm install @modelcontextprotocol/client (or server). (2) npm reads the client's dependencies, resolves @modelcontextprotocol/core transitively, and it lands in node_modules on every install — directly contradicting "not required by client / server". (3) At runtime the installed client's dist chunks contain live import { … } from '@modelcontextprotocol/core/internal' statements, so the dependency is real at module-resolution level, not just in the manifest. A grep over docs/ and package READMEs confirms this is the only surviving occurrence of the old "not required / never depend / never install" framing after 68688c9.\n\nWhy it matters beyond mere staleness. The "install it only if…" framing in the v1→v2 migration guide — the doc most upgrading users actually read — invites users to pin their own core version alongside client/server. Because core/client/server/server-legacy now release as a changesets fixed group with exact pins, a version-skewed self-pinned core resolved in place of the paired one fails at import time with ERR_PACKAGE_PATH_NOT_EXPORTED (the PR description names this failure mode). So the sentence steers readers toward exactly the configuration the fixed group and exact pins were added to prevent.\n\nWhy nothing in the PR catches it. The drift guards added here (coreBoundary.test.ts, schemaShims.test.ts, packageTopologyPins.test.ts) pin build artifacts and manifests, not prose. The doc-topology fix in 68688c9 was scoped to the two files the earlier comment named, and the migration guide was not in the diff.\n\nFix. A one-sentence reword at upgrade-to-v2.md:1328 matching the corrected language in packages.md/wire-schemas.md — e.g.: "It is runtime-neutral (its only dependency is zod). client / server resolve their shared schema graph from it at runtime, so it already arrives transitively in your tree; add it to your own dependencies only when you import the raw schemas directly." The runtime-neutral / zod-only half of the claim remains true and should stay.\n\nSeverity. Nit: doc-only — nothing breaks at runtime if merged as-is, and the "only import it directly if you need raw schemas" advice half remains sound. But it is the same class of stale prose the author already accepted and fixed twice in this PR, so finishing the sweep is cheap and worthwhile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant